refactor(ci): inline bun setup, gate cache on snapshot#281
Merged
Conversation
The composite previously used actions/cache@v5 (full) which auto-saves at job end, allowing PR-controlled code to write into the cache before save. Use actions/cache/restore@v5 for reads and gate actions/cache/save@v5 on github.event_name != 'pull_request' so only trusted runs (push to main, workflow_call from release.yml) populate the cache.
- Gate the save step on the restore step's cache-hit output so we do not warn-spam when the key already exists. - Tighten the trust gate from github.event_name != 'pull_request' to github.event_name == 'push' so issue_comment (!snapshot) runs do not save the cache. Snapshot CI checks out PR-author-controlled code; only push-to-main is a structurally trusted save context.
Replaces inline oven-sh/setup-bun@v2 + actions/cache(/restore)@v5 + bun install steps with a single ./.github/actions/setup-bun reference. Combined with the composite's split restore/save and the github.event_name == 'push' trust gate, this resolves the 9 CodeQL alerts (actions/cache-poisoning/poisonable-step #29-#37) in ci.yml. The test-e2e job is unchanged.
Contributor
Author
|
Stack: wyattjoh/refactor-setup-bun-cache Part of a stacked PR chain. Do not merge manually. |
|
wyattjoh
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughThe setup-bun composite action now documents only "none" and "restore" and uses a restore-only cache step when inputs.cache == "restore". CI jobs (Build, Lint, Test, E2E) gate Bun cache restores on github.event_name != 'issue_comment' and continue to run bun install. Added warm-bun-cache.yml to populate the Bun install cache on pushes to main. Several workflows (notify-failure, notify-release, release jobs) now use oven-sh/setup-bun@v2 with explicit cache restore and bun install steps. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/actions/setup-bun/action.yml (1)
30-40: ⚡ Quick winMake the save-gate comment match the actual condition.
The comment says this saves “only on push to main,” but the guard only checks
github.event_name == 'push'. Either add the branch check or relax the wording so future hardening work is based on the real behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/actions/setup-bun/action.yml around lines 30 - 40, The comment above the "Save Bun cache" step is misleading: it says "Save only on push to main" but the actual if uses inputs.cache, github.event_name == 'push', and steps.bun-cache.outputs.cache-hit != 'true' (no branch check). Either update the plain-text comment to reflect the real guard (mentioning inputs.cache == 'read-write', github.event_name == 'push', and skip-on-cache-hit) or change the if expression to include a branch restriction (e.g., add github.ref == 'refs/heads/main') so the behavior matches the comment; adjust the comment or the if on the "Save Bun cache" step accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/actions/setup-bun/action.yml:
- Around line 30-40: The comment above the "Save Bun cache" step is misleading:
it says "Save only on push to main" but the actual if uses inputs.cache,
github.event_name == 'push', and steps.bun-cache.outputs.cache-hit != 'true' (no
branch check). Either update the plain-text comment to reflect the real guard
(mentioning inputs.cache == 'read-write', github.event_name == 'push', and
skip-on-cache-hit) or change the if expression to include a branch restriction
(e.g., add github.ref == 'refs/heads/main') so the behavior matches the comment;
adjust the comment or the if on the "Save Bun cache" step accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d0d046bf-8ca5-43a0-8b28-95fa159e0fbf
📒 Files selected for processing (2)
.github/actions/setup-bun/action.yml.github/workflows/ci.yml
The setup-bun composite previously contained a conditional cache save step. Even gated to push contexts, the save action's structural presence in a job reachable from a workflow_call that ultimately traces back to issue_comment (the !snapshot path) is enough for CodeQL's cache-poisoning rule to flag every step in the job. Move the cache save into a dedicated warm-bun-cache.yml workflow triggered only by push to main. The composite becomes restore-only and its read-write mode is removed (no callers used it after this refactor's build job switched to restore). PR runs read main's warmed cache via restore-keys fallback; the warmup workflow rebuilds the cache whenever bun.lock changes on main.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/warm-bun-cache.yml (1)
10-12: ⚡ Quick winInclude setup-bun action changes in warm-cache triggers.
Line 10-12 path filters currently skip
.github/actions/setup-bun/action.yml; if cache key/path logic changes there, warm-up may not run after merge and CI stays cold untilbun.lockchanges.Suggested diff
on: push: branches: [main] paths: - bun.lock + - .github/actions/setup-bun/action.yml - .github/workflows/warm-bun-cache.yml🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/warm-bun-cache.yml around lines 10 - 12, The workflow's paths filter under the "paths" key only lists bun.lock and the warm-bun-cache workflow itself, so changes to the setup-bun action won't trigger the warm cache; update the "paths" array to include the setup-bun action file so edits to that action will retrigger the warm-cache job (add the setup-bun action entry to the paths list next to bun.lock and .github/workflows/warm-bun-cache.yml).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/warm-bun-cache.yml:
- Around line 10-12: The workflow's paths filter under the "paths" key only
lists bun.lock and the warm-bun-cache workflow itself, so changes to the
setup-bun action won't trigger the warm cache; update the "paths" array to
include the setup-bun action file so edits to that action will retrigger the
warm-cache job (add the setup-bun action entry to the paths list next to
bun.lock and .github/workflows/warm-bun-cache.yml).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 250e1e85-6e45-4b35-be19-7e125e9a3e17
📒 Files selected for processing (3)
.github/actions/setup-bun/action.yml.github/workflows/ci.yml.github/workflows/warm-bun-cache.yml
Removes the .github/actions/setup-bun composite action and inlines its operations (oven-sh/setup-bun@v2, optional actions/cache/restore@v5, bun install --frozen-lockfile) at all eight call sites. In ci.yml's build, lint, test, and test-e2e jobs, the cache restore is gated on github.event_name != 'issue_comment' so the snapshot path (which checks out PR-author-controlled code in the default branch's privileged context via release.yml's workflow_call) cannot read the cache. Other workflows (release.yml versioning/canary-version, notify-failure, notify-release) inline directly without a gate because they only run on trusted triggers.
Both notify-failure and notify-release run on push-to-main triggered release workflows where the cache is already warm; adding the restore step avoids a cold install on every notification job.
wyattjoh
changed the title
rafa-thayto
approved these changes
May 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Drops the
.github/actions/setup-buncomposite action and inlines its three operations (oven-sh/setup-bun@v2,actions/cache/restore@v5,bun install --frozen-lockfile) at all eight call sites. Inci.yml'sbuild,lint,test, andtest-e2ejobs, the cache restore step is gated ongithub.event_name != 'issue_comment'so the snapshot path — which checks out PR-author-controlled code in the default branch's privileged context viarelease.yml'sworkflow_call— never consumes the cache. The other workflows (release.ymlversioning + canary-version,notify-failure,notify-release) inline directly without a gate because they only run on trusted triggers. Cache population continues to live in the standalonewarm-bun-cache.ymlworkflow that runs solely on push tomain.Test plan
bun run format:check,bun run lint,bun run typecheck,bun run test).actions/cache-poisoning/poisonable-stepalerts.main,Warm Bun cacheruns and the next PR build reads the warmed cache.